Skip to content

Introduce a translation:check-missing command #403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Feb 21, 2020
Merged

Introduce a translation:check-missing command #403

merged 6 commits into from
Feb 21, 2020

Conversation

nicwortel
Copy link
Contributor

@nicwortel nicwortel commented Feb 7, 2020 •
edited
Loading

Fixes #388.

@nicwortel
Copy link
Contributor Author

I don't believe the PHPStan error is introduced by my pull request.

-- ------------------------------------------------------------------------- 
    Error                                                                    
-- ------------------------------------------------------------------------- 
    Ignored error pattern #^Call to method getValue\(\) on an unknown class  
    Symfony\\Component\\Translation\\DataCollector\\Data\.$# in path         
    /github/workspace/Controller/SymfonyProfilerController.php was not       
    matched in reported errors.                                              
-- -------------------------------------------------------------------------

I believe this is caused by an ignored error (in phpstan-baseline.neon) that is no longer reported. I could remove the offending lines from phpstan-baseline.neon.

@nicwortel
Search for a shorter string in the test
aed52a4 
This prevents issues in CI with a smaller console width.
@Guite
Copy link
Contributor

Guite commented Feb 9, 2020

I could remove the offending lines from phpstan-baseline.neon.

Looks like it's removed as part of #396 here: https://github.com/php-translation/symfony-bundle/pull/396/files#diff-cbcd91b5fa0a393ebd972f66cc47dc90

@nicwortel
Copy link
Contributor Author

@Guite you are right. #396 is closed but #395 has fixed the build, so after merging master all checks are green.

My PR is now ready to be reviewed.

welcoMattic
welcoMattic reviewed Feb 18, 2020
View reviewed changes
Copy link
Member

@welcoMattic welcoMattic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except the name of the command, LGTM 👍 thanks for your work!

@welcoMattic
Copy link
Member

@php-translation/deciders ok for you? We could target a 0.12 with this new command?

@nicwortel nicwortel changed the title Introduce a translation:check command Introduce a translation:check-missing command Feb 18, 2020
odolbeau
odolbeau reviewed Feb 18, 2020
View reviewed changes
Command/CheckMissingCommand.php
protected function configure(): void
{
$this
->setName(self::$defaultName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not needed when defining the static property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you are right, the Symfony documentation example also doesn't call setName() anymore.

However, I copied this from the other commands in this bundle which also call ->setName(self::$defaultName). So perhaps this should be fixed for all commands at once in a separate PR.

@nicwortel
Copy link
Contributor Author

Are we waiting for more approvals, or is there anything I should do to get this merged? :)

@welcoMattic welcoMattic merged commit 9112826 into php-translation:master Feb 21, 2020
@welcoMattic welcoMattic mentioned this pull request Feb 21, 2020
Merged
@welcoMattic
Copy link
Member

@nicwortel it's merged, we will release the 0.12 as soon as possible 😉

@nicwortel nicwortel deleted the check-command branch February 21, 2020 15:45
@nicwortel
Copy link
Contributor Author

Nice 👍 I will try to find some time to create a PR for https://github.com/php-translation/documentation.

@welcoMattic
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@odolbeau odolbeau odolbeau approved these changes

@welcoMattic welcoMattic welcoMattic approved these changes

@rvanlaak rvanlaak rvanlaak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a command to check if all extractable translations exist in the translation files
5 participants
@nicwortel @Guite @welcoMattic @odolbeau @rvanlaak